Skip to content

Add Basic examples that can be included in README#64

Merged
sharpener6 merged 21 commits intofinos:mainfrom
gxuu:add_more_examples
Apr 1, 2025
Merged

Add Basic examples that can be included in README#64
sharpener6 merged 21 commits intofinos:mainfrom
gxuu:add_more_examples

Conversation

@gxuu
Copy link
Contributor

@gxuu gxuu commented Mar 25, 2025

PR working on adding more examples of using scaler

gxu and others added 9 commits March 26, 2025 01:42
All examples will be in this directory later

Signed-off-by: gxu <georgexu420@gmail.com>
Read readme for more information

Signed-off-by: gxu <georgexu420@gmail.com>
This make sure all examples are checked when tests are triggered

Signed-off-by: gxu <georgexu420@gmail.com>
Signed-off-by: gxu <georgexu420@gmail.com>
Signed-off-by: gxu <georgexu420@gmail.com>
Signed-off-by: gxu <georgexu420@gmail.com>
Signed-off-by: gxu <georgexu420@gmail.com>
Signed-off-by: gxu <georgexu420@gmail.com>
@gxuu gxuu marked this pull request as ready for review March 28, 2025 12:51
Signed-off-by: gxu <georgexu420@gmail.com>
@gxuu gxuu changed the title WIP More examples on how to use scaler Basic examples that can be included in README Mar 28, 2025
@gxuu gxuu changed the title Basic examples that can be included in README Add Basic examples that can be included in README Mar 28, 2025
Copy link
Collaborator

@rafa-be rafa-be left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few changes I'd suggest:

  • Could we merge some of these examples?
    • simple_client, send_object, disconnect_client and map_client share 90% of the code, maybe you can merge them as a single file with basic use cases on how to use the Scaler's client.
    • graphtask_nested_client can be a little bit simplified, and possibly added to graphtask_client, to demonstrate that you can submit graph tasks from within a task.
    • simple_scheduler and simple_cluster could be merged in a single example showing the different ways of setting a Scaler cluster.
  • What is the text wrapping setting for comments? I've the feeling that these are not consistent.
  • Maybe use a docstring when describing the file's content, like I did here: https://github.com/rafa-be/parfun/blob/main/benchmarks/california_housing.py

gxu added 3 commits March 28, 2025 23:09
Signed-off-by: gxu <georgexu420@gmail.com>
Use docstring and line wrap comment with 120 col.

Signed-off-by: gxu <georgexu420@gmail.com>
Signed-off-by: gxu <georgexu420@gmail.com>
Copy link
Contributor

@magniloquency magniloquency left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor comments about making documentation language more consistent and concise, plus a couple of questions.

gxuu and others added 6 commits March 30, 2025 00:15
Co-authored-by: magniloquency <197707854+magniloquency@users.noreply.github.com>
Signed-off-by: gxu <69813939+gxuu@users.noreply.github.com>
Signed-off-by: gxu <georgexu420@gmail.com>
Signed-off-by: gxu <georgexu420@gmail.com>
Co-authored-by: magniloquency <197707854+magniloquency@users.noreply.github.com>
Signed-off-by: gxu <69813939+gxuu@users.noreply.github.com>
Co-authored-by: magniloquency <197707854+magniloquency@users.noreply.github.com>
Signed-off-by: gxu <69813939+gxuu@users.noreply.github.com>
Signed-off-by: gxu <georgexu420@gmail.com>
sharpener6
sharpener6 previously approved these changes Mar 31, 2025
@sharpener6
Copy link
Collaborator

@gxuu , please fix linter issue, always run the same linter git flow script

Signed-off-by: gxu <georgexu420@gmail.com>
@gxuu gxuu requested a review from sharpener6 March 31, 2025 19:31
@sharpener6
Copy link
Collaborator

A few changes I'd suggest:

* Could we merge some of these examples?
  
  * `simple_client`, `send_object`, `disconnect_client` and `map_client` share 90% of the code, maybe you can merge them as a single file with basic use cases on how to use the Scaler's client.
  * `graphtask_nested_client` can be a little bit simplified, and possibly added to `graphtask_client`, to demonstrate that you can submit graph tasks from within a task.
  * `simple_scheduler` and `simple_cluster` could be merged in a single example showing the different ways of setting a Scaler cluster.

* What is the text wrapping setting for comments? I've the feeling that these are not consistent.

* Maybe use a docstring when describing the file's content, like I did here: https://github.com/rafa-be/parfun/blob/main/benchmarks/california_housing.py

I understand the refactoring purpose, but my thinking about this is slightly different, since this is for example, the goal for those should be executable and able to copy for each example, so it's better to be in one snippet

@gxuu gxuu requested a review from rafa-be March 31, 2025 23:45
@sharpener6 sharpener6 merged commit 38088b9 into finos:main Apr 1, 2025
5 checks passed
@gxuu gxuu deleted the add_more_examples branch April 2, 2025 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants